fix(FIX-017): Ctrl+C in SSH interrupts remote process instead of dropping connection#42
Merged
Merged
Conversation
…ping connection
Pressing Ctrl+C while SSHed into a remote host (with any remote process
running — sleep, vim, htop, an editor) was dropping the SSH connection
entirely with "Connection to <host> closed", losing unsaved state in
remote editors and aborting long-running remote work. The originally-
expected behavior — interrupt the remote process, keep the session
alive — is now delivered. Target UX: "Ctrl+C interrupts whatever is
running, regardless of whether you're local or in a remote session"
(Windows Terminal parity).
Root cause: an unconditional `kill(-pgid, SIGINT)` in
`SessionManager::send_sigint` (`crates/forgetty-session/src/manager.rs`).
The kill was added by BUG-001 as a fallback for raw-mode apps that
swallow `0x03` (Node.js, pm2). When SSH is the foreground process,
the kill landed on the local ssh client itself; OpenSSH installs no
SIGINT handler in interactive mode, so the default disposition
(terminate) killed it before it could forward the byte to the remote.
Three layered changes, satisfying AD-007 (daemon=byte-pipe), AD-008
(clients-own-VT), AD-009 (no-polling-on-PTY-hot-path), AD-010 (raw-
bytes-on-wire), and AD-011 (daemon-always-runs):
1. `crates/forgetty-session/src/manager.rs`: skip `kill(-pgid)` when
the foreground pgrp leader is a known signal-forwarder. Detected
via `/proc/<pgid>/comm` against a hardcoded allowlist of
`{ ssh, mosh-client, telnet, rsh }`. The kill remains for any
non-forwarder (Node.js, pm2, anything that needs the BUG-001
fallback path).
2. `crates/forgetty-pty/src/process.rs` + `forgetty-session` +
`forgetty-socket`: read `c_cc[VINTR]` from the master fd's termios
via `libc::tcgetattr` and write that byte instead of hardcoded
`0x03`. The remote PTY inherits VINTR from the local PTY via
ssh's `pty-modes` payload, so the byte we write matches whatever
the remote line discipline expects as the interrupt char.
Local-VINTR-remapped setups (e.g. `stty intr ^X`) now interrupt
correctly through SSH; default-VINTR users are unaffected (still
write `0x03`).
3. `crates/forgetty-gtk/src/terminal.rs`: suppress the line-discipline
ECHOCTL echo of the VINTR byte for 150 ms after Ctrl+C. Without
this, the next ~150 ms of output would show e.g. `^X` or `^C`
printed by the remote PTY's echo machinery — visually mismatched
with what the user pressed. Mirrors BUG-003's BEL-suppression
pattern: client-side filter drops the first `^<letter>` caret-
form pair in the byte stream before it reaches the VT parser.
11 new unit tests: 3 in `forgetty-session` (allowlist matcher
correctness for ssh/mosh-client/telnet/rsh vs node/python3/bash/etc.,
plus an I/O wrapper test for missing-pid → false), 1 in `forgetty-pty`
(`vintr()` returns 0x03 on a freshly-spawned PTY confirming the
`tcgetattr` plumbing), 7 in `forgetty-gtk` (the echo-suppression
filter: matched-and-cleared, default `^C` echo dropped, DEL `^?`
dropped, no-suppression passthrough, expired-deadline passthrough,
non-control letter not dropped, only-first-match semantics).
All cargo checks clean (fmt, check, clippy `-D warnings`, test, build
--release). 207 workspace tests pass (was 196 + 11 new). Zero
findings across three audit-skill passes (simplify, audit-rust,
audit-security). Performance: two cold-path syscalls per Ctrl+C
keypress (`/proc/<pgid>/comm` ~1 µs + `tcgetattr` ~1 µs); ECHOCTL
filter is `O(chunk)` for ≤150 ms after Ctrl+C; AD-009 preserved.
Criterion benches at-noise vs baseline (absolute values well under
SPEC ceilings).
QA scores: 9/10/9/9/10 (Completeness / Functionality / Code quality /
Robustness / Performance impact). All ACs PASS, including AC-10
(round-2 dynamic VINTR pickup + ECHOCTL echo suppression). Vick
verified end-to-end with both Ctrl+C in SSH and the multi-press
case at the prompt; reported "Fully working and perfect!!".
FIX-018 (wheel scroll broken inside SSH, reported in the same message
as FIX-017) is explicitly out of scope — separate code path (wheel
handler vs key handler), separate Planner cycle. Tracked in BACKLOG.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Pressing Ctrl+C while SSHed into a remote host was dropping the SSH connection entirely (
Connection to <host> closed), losing unsaved state in remote editors and aborting long-running remote work. The originally-expected behavior — interrupt the remote process, keep the session alive — is now delivered. Three layered changes deliver Windows-Terminal-parity UX: "Ctrl+C interrupts whatever is running, regardless of whether you're local or in a remote session."Root cause
SessionManager::send_sigintincrates/forgetty-session/src/manager.rscalledkill(-pgid, SIGINT)unconditionally for the foreground pgrp. BUG-001 added it as a fallback for raw-mode apps that swallow0x03(Node.js, pm2). But when ssh is the foreground process, the kill landed on the local ssh client itself; OpenSSH installs no SIGINT handler in interactive mode, so the default disposition (terminate) killed it before it could forward the byte to the remote.QA round 1 then revealed a deeper issue: the bare
0x03byte we wrote didn't match the remote PTY'sVINTRsetting. Vick's local PTY hasintr = ^X(system-wide on his machine — visible in Ghostty too), and ssh'stty_make_modes()forwards localc_ccto the server viapty-req, so the remote PTY inheritsVINTR = ^Xtoo. The line discipline only fires SIGINT when the configuredVINTRbyte arrives —0x03was just echoed and discarded. Round-2 main added dynamicVINTRpickup so the byte we write matches whatever the remote line discipline expects.QA round 2 then surfaced a cosmetic concern: the remote ECHOCTL echo of the VINTR byte (
^Xfor Vick,^Cfor default users) rendered in the terminal after every Ctrl+C — visually mismatched with the key pressed. Round-2 follow-up added a 150 ms client-side filter that drops the first caret-form echo pair after Ctrl+C.How the fix works
Three coordinated landings:
Skip kill for signal-forwarders (
forgetty-session/src/manager.rs). Insend_sigint, before firingkill(-pgid, SIGINT), check the foreground pgrp leader's/proc/<pgid>/commagainst a hardcoded allowlist of{ ssh, mosh-client, telnet, rsh }. The kill remains for any non-forwarder so BUG-001's interruption of raw-mode swallowers (Node.js, pm2) still works.Dynamic
VINTRbyte (forgetty-pty/src/process.rs+forgetty-session+forgetty-socket).PtyProcess::vintr()readsc_cc[VINTR]vialibc::tcgetattron the master fd, exposed viaSessionManager::intr_byte(id).handle_send_sigintnow writessm.intr_byte(id).unwrap_or(0x03)instead of hardcoded&[0x03]. The byte we write matches whatever the local — and therefore remote, via ssh's pty-modes forwarding — line discipline treats as the interrupt char.ECHOCTL echo suppression (
forgetty-gtk/src/terminal.rs). NewTerminalState::suppress_intr_echo_until: Option<Instant>field, set tonow + 150msin the Ctrl+C key handler. New module-private helperdrop_first_caret_form_echo()scans incomingDaemonOutputMessage::Datachunks for the first^<letter>printable pair (caret-form ECHOCTL echo of any control char in0x00..=0x1for DEL0x7f) and removes it before feeding to the VT parser. "Consume on first drop" semantics bound the over-filter blast radius to at most one pair per Ctrl+C press.The fix never reaches into Forgetty's daily-driver PTY state (all QA inside
./launch-dev.shsandbox). Architectural invariants AD-007 through AD-011 verified preserved in audit passes.Tests
11 new unit tests, 207 workspace tests pass (was 196 + 11 new):
forgetty-session— 3 tests covering the allowlist matcher (ssh / mosh-client / telnet / rshmatch;node / python3 / bash / zsh / vim / \"\" / sshd / my-ssh-wrapperdo not match) and the I/O wrapper's safe-default on a non-existent pid.forgetty-pty— 1 test thatvintr()returnsSome(0x03)on a freshly-spawned PTY (the kernel default; confirms thetcgetattrplumbing works end-to-end).forgetty-gtk— 7 tests fordrop_first_caret_form_echo:^Xdropped and flag cleared, default^Cdropped, DEL^?dropped, no-suppression passthrough, expired-deadline passthrough + flag clear, lowercase^anot dropped, only-first-match semantics.Toolchain:
cargo fmt --check,cargo check --workspace,cargo clippy --workspace -- -D warnings,cargo test --workspace --exclude forgetty-gtk+cargo test -p forgetty-gtk,cargo build --release— all clean. Zero findings across three audit-skill passes (simplify, audit-rust, audit-security). Daemon smoke test (timeout 5 ./target/release/forgetty-daemon --foreground) — clean exit on SIGTERM.Performance: two cold-path syscalls per Ctrl+C keypress (~3 µs total). ECHOCTL filter is
O(chunk)for ≤150 ms after Ctrl+C; zero allocation in the no-match case (returnsCow::Borrowed). Criterion benches (daemon_hotpath) at-noise vs baseline. AD-009 (no polling on PTY hot path) preserved.Files
Out of scope
docs/harness/BACKLOG.md.app.rs:6320("Copy Selection — Ctrl+C" labels the wrong accel; the actual registered accel is<Control><Shift>c). Pre-existing doc drift, no functional impact.Test plan checklist
cargo fmt --all --checkcargo check --workspacecargo clippy --workspace -- -D warningscargo test --workspace --exclude forgetty-gtk(196 → 207 with the 11 new tests)cargo test -p forgetty-gtk(the new echo-filter tests)cargo build --releasesleep 100+ Ctrl+C → interrupts, session stays opensleep 100+ Ctrl+C → still interruptsVINTR = ^X+ Ctrl+C → interrupts AND no^Xecho printed^XechoesVick reported: "Fully working and perfect!!"